Skip to content

Multiselect filtering for "no values" in nblocks column #1106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 2, 2025

Conversation

BenjaminCharmes
Copy link
Contributor

Closes #1104

Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.19%. Comparing base (cc31504) to head (be82424).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1106   +/-   ##
=======================================
  Coverage   71.19%   71.19%           
=======================================
  Files          64       64           
  Lines        4236     4236           
=======================================
  Hits         3016     3016           
  Misses       1220     1220           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

cypress bot commented Apr 9, 2025

datalab    Run #3251

Run Properties:  status check passed Passed #3251  •  git commit 0908366deb ℹ️: Merge be82424b4331661812ada11edb1556995a8f132b into cc31504bd12ad8e2a6b7e873f919...
Project datalab
Branch Review bc/filter-noblock
Run status status check passed Passed #3251
Run duration 07m 48s
Commit git commit 0908366deb ℹ️: Merge be82424b4331661812ada11edb1556995a8f132b into cc31504bd12ad8e2a6b7e873f919...
Committer Ben Charmes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 501
View all changes introduced in this branch ↗︎

@jdbocarsly
Copy link
Member

I haven't tried on this branch, but I was also noticing that "match any" seems to always match samples with 0 blocks. Is that easy to fix?

@ml-evs
Copy link
Member

ml-evs commented Apr 9, 2025

This is doing something a bit strange on my local instance, filtering for "match all" -> "no blocks" still leaves some entries that have blocks. Perhaps relatedly, on main and this PR, if I try to simply sort in ascending order by the blocks column, it starts with the entries with 1 block rather than 0.

@BenjaminCharmes
Copy link
Contributor Author

BenjaminCharmes commented Apr 10, 2025

Thank for the review @jdbocarsly & @ml-evs !

  • For Josh, do you mean with "Math any" and the second dropdown empty ? (With just "any" and no blocks selected ?). If yes, now that we add "No blocks" in the dropdown, I guess it make sense that "Match any" also give sample with no block ? But yes, before, it should only have filter samples with blocks.
  • I can't reproduce the first behaviour Matt describes with "match all -> no blocks"
  • For the second behaviour you describe, it's probably because the box is empty (and doesn't have ‘0’) when there are no blocks. I can look for a way to change that

I'm going to take a closer look at all this!

@ml-evs
Copy link
Member

ml-evs commented May 2, 2025

Realised what is causing this behaviour: when you copy a sample that has blocks, the block is copied identically and then the API summary doesn't include it even though nblocks: 1 -- weird bug we need to fix

@ml-evs ml-evs force-pushed the bc/filter-noblock branch from 2fbb0e3 to be82424 Compare May 2, 2025 22:06
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well, now that we figured out that weird bug in the API!

@ml-evs ml-evs merged commit e948afc into main May 2, 2025
17 checks passed
@ml-evs ml-evs deleted the bc/filter-noblock branch May 2, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possibility of filtering multiselect for "no values"
3 participants